Skip to content

Patched several memory leaks#17

Closed
MythicPalette wants to merge 1 commit into
Streamerbot:mainfrom
MythicPalette:main
Closed

Patched several memory leaks#17
MythicPalette wants to merge 1 commit into
Streamerbot:mainfrom
MythicPalette:main

Conversation

@MythicPalette

Copy link
Copy Markdown

I noticed abandoned listeners due to the following --warn-trace output:

(node:188671) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 message listeners added to [WebSocket]. MaxListeners is 10. Use emitter.setMaxListeners() t
o increase limit
    at genericNodeError (node:internal/errors:985:15)
    at wrappedFn (node:internal/errors:539:14)
    at _addListener (node:events:582:17)
    at WebSocket.addListener (node:events:600:10)
    at WebSocket.addEventListener (node_modules/ws/lib/event-target.js:249:12)
    at withTimeout.timeout.timeout (node_modules/@streamerbot/client/dist/index.js:1079:20)
    at new Promise (<anonymous>)
    at StreamerbotClient.request (node_modules/@streamerbot/client/dist/index.js:1078:40)
    at StreamerbotClient.getUserGlobal (node_modules/@streamerbot/client/dist/index.js:1614:33)
    at F (apps/server/dist/server.js:1:7050)
  • note: I did modify the shown file paths as they contained my private file structure. The only modifications made were removing the path preceding node_modules

Searching node_modules/@streamerbot/client/dist/index.js, I located several instances where anonymous function listeners were being assigned to this.socket and then abandoned.

I do apologize, a couple lines in my code got hit by reformatting so that's why a few of the previously single lines are multiline now.

@Whipstickgostop

Copy link
Copy Markdown
Member

Sorry I've finally had a chance to review this.

The issue here isn't anonymous listeners, rather that the fallback ws package, used in Node.js environments, does not actually fully support the full browser WebSocket specification, and lacks support for the signal configuration option. This resulted in listeners that were being properly cleaned up in browser environments, but were indeed being left dangling in Node.js environments.

In a soon to be published release, I have migrated this library to utilize the native WebSocket client, which is now available by default in Node.js 24 or later. This allows me to drop the additional ws and bufferutil dependencies entirely, as the native client now has full parity with the WebSocket class traditionally found in browsers.

@MythicPalette

Copy link
Copy Markdown
Author

Awesome. Glad to hear it. Also, I did not know that about the node ws package. Good information to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants